HDDS-14036. StreamRead: Make preReadSize, responseDataSize and read timeout configurable by Russole · Pull Request #9461 · apache/ozone

Russole · 2025-12-08T07:33:23Z

What changes were proposed in this pull request?

This change makes the fixed values used by StreamBlockInputStream for
preReadSize, responseDataSize, and the streaming read timeout configurable
via OzoneClientConfig and ozone-default.xml.
This allows users and operators to tune stream-read performance based on workload
characteristics and cluster latency.

Currently, StreamBlockInputStream uses the following fixed values:

preReadSize = 32MB
responseDataSize = 1MB
a fixed 10-second streaming read timeout (poll(10, TimeUnit.SECONDS))

These defaults work for general cases but may not be optimal for all environments, particularly under higher latency conditions or workloads that benefit from larger streaming buffers or more fine-grained timeout control.

This PR introduces three new client-side configuration keys to make these parameters fully configurable:

ozone.client.stream.read.pre-read-size
ozone.client.stream.read.response-data-size
ozone.client.stream.read.timeout

Changes include:

Adding the new configuration keys and default values to ozone-default.xml.
Introducing corresponding fields and getters in OzoneClientConfig.
Updating StreamBlockInputStream to use values from OzoneClientConfig instead of fixed constants.
Replacing the previous fixed 10-second timeout in StreamingReader.poll() with a configurable timeout.
Adding constants to OzoneConfigKeys for consistency and discoverability.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-14036

How was this patch tested?

GitHub Actions CI for my fork ran successfully with all checks passing.

…imeout configurable

sreejasahithi · 2025-12-08T12:08:56Z


    ByteBuffer readFromQueue() throws IOException {
-      final ReadBlockResponseProto readBlock = poll(10, TimeUnit.SECONDS);
+      final ReadBlockResponseProto readBlock = poll(readTimeoutMs, TimeUnit.SECONDS);


Thanks @Russole for working on this, left few comments

Because readTimeoutMs is in milliseconds, you must pass TimeUnit.MILLISECONDS so the timeout isn’t misinterpreted as seconds

Suggested change

final ReadBlockResponseProto readBlock = poll(readTimeoutMs, TimeUnit.SECONDS);

final ReadBlockResponseProto readBlock = poll(readTimeoutMs, TimeUnit.MILLISECONDS);

sreejasahithi · 2025-12-08T12:09:05Z

+  private int streamReadResponseDataSize = 1 << 20;
+
+  @Config(key = "ozone.client.stream.read.timeout",
+      defaultValue = "1000",


The default value should be 10000 in order to match with the ozone-default.xml

adoroszlai

Thanks @Russole for working on this.

Please don't also add the config to OzoneConfigKeys and ozone-default.xml.

Please add test cases to verify custom config is applied.

adoroszlai · 2025-12-08T12:49:36Z

+      defaultValue = "1000",
+      type = ConfigType.INT,


Please use ConfigType.TIME and "10s".

adoroszlai · 2025-12-08T12:51:30Z

+      type = ConfigType.INT,
+      tags = {ConfigTag.CLIENT},
+      description = "Timeout in ms for receiving streaming read responses.")
+  private int streamReadTimeoutMs = 10_000;


Plese use Duration instead of int and drop Ms from name.

adoroszlai · 2025-12-08T12:52:11Z

+  public int getStreamReadTimeoutMs() {
+    return streamReadTimeoutMs;


Return Duration and drop Ms from name.

jojochuang · 2025-12-08T17:12:45Z

@yandrey321

Russole · 2025-12-10T12:27:36Z

Many thanks to @adoroszlai, @sreejasahithi, and @jojochuang for the reviews. I've addressed the comments — please let me know if any further changes are needed.

adoroszlai

Thanks @Russole for updating the patch.

adoroszlai · 2025-12-10T12:30:12Z

+  public static final String
+      OZONE_CLIENT_STREAM_READ_PRE_READ_SIZE =
+      "ozone.client.stream.read.pre-read-size";
+  public static final long
+      OZONE_CLIENT_STREAM_READ_PRE_READ_SIZE_DEFAULT =
+      32L << 20;
+
+  public static final String
+      OZONE_CLIENT_STREAM_READ_RESPONSE_DATA_SIZE =
+      "ozone.client.stream.read.response-data-size";
+  public static final int
+      OZONE_CLIENT_STREAM_READ_RESPONSE_DATA_SIZE_DEFAULT =
+      1 << 20;
+
+  public static final String
+      OZONE_CLIENT_STREAM_READ_TIMEOUT =
+      "ozone.client.stream.read.timeout";
+  public static final int
+      OZONE_CLIENT_STREAM_READ_TIMEOUT_DEFAULT =
+      10_000;
+


Sorry, if my original comment was confusing. Please remove these, too.

Suggested change

public static final String

OZONE_CLIENT_STREAM_READ_PRE_READ_SIZE =

"ozone.client.stream.read.pre-read-size";

public static final long

OZONE_CLIENT_STREAM_READ_PRE_READ_SIZE_DEFAULT =

32L << 20;

public static final String

OZONE_CLIENT_STREAM_READ_RESPONSE_DATA_SIZE =

"ozone.client.stream.read.response-data-size";

public static final int

OZONE_CLIENT_STREAM_READ_RESPONSE_DATA_SIZE_DEFAULT =

1 << 20;

public static final String

OZONE_CLIENT_STREAM_READ_TIMEOUT =

"ozone.client.stream.read.timeout";

public static final int

OZONE_CLIENT_STREAM_READ_TIMEOUT_DEFAULT =

10_000;

adoroszlai · 2025-12-10T12:31:32Z

+
+    Token<OzoneBlockTokenIdentifier> token = null;
+    // Mock XceiverClientFactory since StreamBlockInputStream requires it in the constructor
+    XceiverClientFactory xceiverClientFactory = Mockito.mock(XceiverClientFactory.class);


nit: please add import static org.mockito.Mockito.mock;

adoroszlai · 2025-12-10T12:35:19Z

+    conf.set("ozone.client.stream.read.pre-read-size", "67108864");
+    conf.set("ozone.client.stream.read.response-data-size", "2097152");
+    conf.set("ozone.client.stream.read.timeout", "5s");
+
+    OzoneClientConfig clientConfig = conf.getObject(OzoneClientConfig.class);


OzoneConfiguration is not required, please create OzoneClientConfig directly and set values using its methods.

OzoneClientConfig clientConfig = new OzoneClientConfig(); clientConfig.set...(...);

adoroszlai · 2025-12-10T12:36:02Z

  }
+
+  @Test
+  public void testCustomStreamReadConfigIsApplied() throws Exception {


This should be a unit test, since it does not require a working cluster. Please move to new test class hadoop-hdds/client/src/test/java/org/apache/hadoop/hdds/scm/storage/TestStreamBlockInputStream.java.

adoroszlai · 2025-12-10T12:41:30Z

+    OzoneClientConfig clientConfig = conf.getObject(OzoneClientConfig.class);
+
+    // Sanity check
+    assertEquals(Duration.ofSeconds(5), clientConfig.getStreamReadTimeout());


To test whether OzoneClientConfig applies the new config keys correctly, add test case in existing TestOzoneClientConfig.

adoroszlai · 2025-12-10T12:59:12Z

+      // Convert Duration -> int seconds for poll(...)
+      final int timeoutSeconds;
+      if (readTimeout == null || readTimeout.isZero() || readTimeout.isNegative()) {
+        timeoutSeconds = 0;
+      } else {
+        long sec = readTimeout.getSeconds();
+        // Prevent overflow if client config is extremely large
+        timeoutSeconds = sec > Integer.MAX_VALUE ? Integer.MAX_VALUE : (int) sec;
+      }
+      final ReadBlockResponseProto readBlock = poll(timeoutSeconds, TimeUnit.SECONDS);


Conversion to seconds is unnecessary, since poll then converts to nanoseconds.

Validation should be done in OzoneClientConfig#validate()

Calculate timeoutNanos in StreamBlockInputStream constructor as config.getStreamReadTimeout().toNanos(). poll parameters can be removed.

adoroszlai · 2025-12-10T13:01:33Z

+  private final int responseDataSize; // Default size is 1 MB
+  private final long preReadSize; // Default size is 32 MB
+  private final Duration readTimeout; // // Default timeout is 10 second


Please remove the comments, they easily get outdated. Default values can be checked in the config.

adoroszlai

Thanks @Russole for updating the patch. LGTM, except two minor items.

adoroszlai · 2025-12-12T21:14:04Z

-    ReadBlockResponseProto poll(int timeout, TimeUnit timeoutUnit) throws IOException {
-      final long timeoutNanos = timeoutUnit.toNanos(timeout);
+    ReadBlockResponseProto poll() throws IOException {
+      final long timeoutNanos = readTimeoutNanos;


nit: remove timeoutNanos and use readTimeoutNanos directly.

adoroszlai · 2025-12-12T21:14:28Z

      "ozone.client.elastic.byte.buffer.pool.max.size";
  public static final String OZONE_CLIENT_ELASTIC_BYTE_BUFFER_POOL_MAX_SIZE_DEFAULT = "16GB";
-
+  


nit: please avoid whiitespace-only change

Suggested change

sreejasahithi

Thanks @Russole for working on this.
LGTM

adoroszlai · 2025-12-13T06:36:15Z

Thanks @Russole for updating the patch. Please note that the whitespace change was made in the wrong line, so now we have 2 changed lines instead of 0. There should be no changes in hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java at all.

Russole · 2025-12-13T09:12:19Z

Thank you @adoroszlai for the reminder. I have removed all whitespace-only changes from hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java.

Russole · 2025-12-13T18:23:25Z

CI failures look flaky and unrelated to this change.
Could you please re-run the failed jobs?

szetszwo

+1 the change looks good.

szetszwo · 2025-12-15T04:22:41Z

@Russole , thanks for working on this!

@adoroszlai , thanks for reviewing this!

Russole added 3 commits December 6, 2025 16:55

HDDS-14036. StreamRead: Make preReadSize, responseDataSize and read t…

abdf854

…imeout configurable

Fix failed unit test

a58f7ae

Merge branch 'master' into HDDS-14036

ea63672

sreejasahithi suggested changes Dec 8, 2025

View reviewed changes

adoroszlai requested changes Dec 8, 2025

View reviewed changes

adoroszlai reviewed Dec 8, 2025

View reviewed changes

Russole added 2 commits December 10, 2025 02:15

Apply fixes based on review feedback

7975ce3

Add comments for clarity

e2538b2

Russole requested review from adoroszlai and sreejasahithi December 10, 2025 12:27

adoroszlai changed the title ~~Hdds 14036. StreamRead: Make preReadSize, responseDataSize and read timeout configurable~~ HDDS-14036. StreamRead: Make preReadSize, responseDataSize and read timeout configurable Dec 10, 2025

Fix findbugs problem

99a2c51

adoroszlai reviewed Dec 10, 2025

View reviewed changes

Russole added 2 commits December 11, 2025 23:57

Update the patch according to reviewer suggestions

9025662

Fix checkstyle

dbf468c

Russole requested a review from adoroszlai December 12, 2025 17:31

adoroszlai reviewed Dec 12, 2025

View reviewed changes

Remove timeoutNanos and WhiteSpace

4befcdf

Russole requested a review from adoroszlai December 13, 2025 01:45

sreejasahithi approved these changes Dec 13, 2025

View reviewed changes

Russole added 2 commits December 13, 2025 16:54

Remove whitespace change

d178732

Restore OzoneConfigKeys to remove whitespace change

c200ae6

adoroszlai approved these changes Dec 13, 2025

View reviewed changes

adoroszlai requested a review from szetszwo December 13, 2025 15:55

szetszwo approved these changes Dec 15, 2025

View reviewed changes

szetszwo merged commit 114d242 into apache:master Dec 15, 2025
82 of 84 checks passed

	final ReadBlockResponseProto readBlock = poll(readTimeoutMs, TimeUnit.SECONDS);
	final ReadBlockResponseProto readBlock = poll(readTimeoutMs, TimeUnit.MILLISECONDS);

		public int getStreamReadTimeoutMs() {
		return streamReadTimeoutMs;

		"ozone.client.elastic.byte.buffer.pool.max.size";
		public static final String OZONE_CLIENT_ELASTIC_BYTE_BUFFER_POOL_MAX_SIZE_DEFAULT = "16GB";

Conversation

Russole commented Dec 8, 2025

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

sreejasahithi Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adoroszlai left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jojochuang commented Dec 8, 2025

Uh oh!

Russole commented Dec 10, 2025

Uh oh!

adoroszlai left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adoroszlai left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sreejasahithi left a comment

Choose a reason for hiding this comment

Uh oh!

adoroszlai commented Dec 13, 2025

Uh oh!

Russole commented Dec 13, 2025

Uh oh!

Russole commented Dec 13, 2025

Uh oh!

szetszwo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

szetszwo commented Dec 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sreejasahithi Dec 8, 2025 •

edited

Loading